-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix all the deprecated-warnings and upgrade django to 1.11 #664
base: django-2.2
Are you sure you want to change the base?
Fix all the deprecated-warnings and upgrade django to 1.11 #664
Conversation
@sks444 If you could make atomic commits, that'd make reviewing your PR (as well as iterating on them later) much easier! ^>^ |
junction/devices/models.py
Outdated
@@ -14,7 +13,7 @@ def expiry_time(expiry_mins=60): | |||
|
|||
|
|||
class Device(TimeAuditModel): | |||
uuid = UUIDField(version=1, hyphenate=True, unique=True, db_index=True) | |||
uuid = models.UUIDField(default=uuid.uuid4, editable=False, unique=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so we'll move to Django's UUIDField and figure out a way to migrate old data to this.
@pradyunsg would appreciate some help around this.
So I replaced the django-uuidfield
with Django's UUIDField which created a new migration. But the old migration of this field still exists which will throw error as the uuidfield
package doesn't exist anymore. What do you think would be the best way to handle this scenario, currently I am commenting out the lines which uses that package in the old migration.
Also, how do I migrate the new migrations, as uuid
column for this model already exists. And while doing so how do I make sure that the old data of that field is migrated to the new one.
I looked it up online but couldn't find anything on how to tackle this problem, some guidance/hints would be great. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so this is resolved, as I got to know that it's better to edit the old migration files to pretend that the field was always a models.UUIDField. That way we won't require any migration. Checkout the answer at Django Forum for more clarification.
Yes, thank you for the suggestion @pradyunsg , I will try to do atomic commits. Actually in this case, when I upgrade django, it starts breaking few other things, so all those fixes should go in one commit right? Or should I have a separate commit for each of the fix? But they are dependent on only one thing, i.e. upgrading django. |
I think a separate commit per fix would be better. |
ddba3a4
to
5707742
Compare
I think this is ready for review now. :) |
@sks444 Why are new migrations created for conferences and proposals ? There is no change in conference or proposal model. |
Also can you rebase your changes on the latest master and solve the linting errors. |
5707742
to
035282b
Compare
@ananyo2012, That is coming from the I moved the migration files to |
So everything seems to be running perfectly, tests are passing and I am interacting with the UI and checking if these changes break something. But as I don't have much experience with the project I might miss few features. @pradyunsg, @ananyo2012, would be great if you could help me with the same. :) |
d527886
to
e70b6b2
Compare
Do your thing Travis CI. |
@sks444 Can you rebase your changes over the latest master ? |
e70b6b2
to
b8dddb9
Compare
Rebased @ananyo2012 |
Note: Putting a hold on the merge until the upgrade compatibility is tested |
@sks444 The CI is failing due to conflicting migrations. Can you please check? |
b8dddb9
to
ad3aea3
Compare
Resolved the migration issue @palnabarun |
This isn't supported on Python 2. @pythonindia/pycon-india-2020-junction Do we care about Python 2 support? If not, let's drop it? |
@sks444 Thank you for resolving that. @pradyunsg We do care about Python 2 at the moment. |
@onlinejudge95 we can not move directly to Django 2 without merging this pr as it would break a lot of things. I have made all the changes here which are required to upgrade to Django 1.11. Although I have tested this on my system, it would very helpful if you could also verify this on yours. So that we could go ahead and merge this and then we can try upgrading to Django 2 and see what breaks. |
@sks444 on it Even I was bumping the versions incrementally it failed for 1.10 for me due to obsolete dependencies. |
@sks444 -- can you please change the target of your changes to |
ad3aea3
to
8d78d59
Compare
Done @palnabarun |
@sks444 @palnabarun Proposing to drop support for |
We will be dropping Python 2.7 support and remove it from before this is
merged
…On Tue, 9 Jun, 2020, 2:15 PM onlinejudge95, ***@***.***> wrote:
@sks444 <https://github.com/sks444> @palnabarun
<https://github.com/palnabarun> Django REST framework has dropped support
for Python2.7 link <https://www.django-rest-framework.org/#requirements>.
I doubt we will be able to pass the installation steps in Travis build due
to this.
Proposing to drop support for Python2.7, what do you say guys?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#664 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQAER3KYLKZQMLQXUN5OSDRVXZBDANCNFSM4MASPRZQ>
.
|
Toward #611
Note: This PR has changes incompatible with Python 2.7 and is a part of the task involving Django 2.2 upgrade
Upgrading django-pagedown to
1.0.6
because the latest version doesn't supportDjango 1.11
, will be upgrading to latest while moving toDjango 2.0
Fix AttributeError: type object 'RadioSelect' has no attribute 'renderer': 1
Use django-markdown-app as django_markdown is not maintained.
Upgrade djangorestframework to latest version which removes the support of filters, so update django-filters to latest and made changes according to the requirements: 1
Upgrade celery to latest version
Droping use of django-uuidfield as Django has it's own UUIDfield: need to figure out a way to handle the migration
Upgrade django-bootstrap3==11.0.0, needs to upgraded to latest while moving to
Django 2
core.context_processors
is moved totemplate.context_processors
Use
user.is_authenticated
as propertyAfter these changes, only deprecated warnings remaining is related to
Django
which I think will vanish when we finally move toDjango 2.0